Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix generated libmsh3.pc file #225

Merged
merged 3 commits into from
Nov 25, 2024
Merged

fix generated libmsh3.pc file #225

merged 3 commits into from
Nov 25, 2024

Conversation

vszakats
Copy link
Contributor

@vszakats vszakats commented Oct 9, 2024

  • fix -I path in Cflags:
  • fix to set the correct prefix: value (replacing the hard-wired one)
  • fix to leave ${} expressions as-is in the template
    (before this patch they were all replaced with empty values)

vszakats added a commit to curl/curl that referenced this pull request Oct 10, 2024
…ustls, wolfSSL

Also:
- detect and add required system libraries for Rustls on macOS and
  non-Windows.
- add Linux CMake jobs for the touched dependencies.
  Caveats:
  - MSH3 generates a broken `libmsh3.pc`, so needs manual config.
    Upstream PR: nibanks/msh3#225
  - Rustls `.pc` file missing, so needs manual config.

An internal change worthy of mention is that we are using the lib path
and name information returned by `pkg-config` as-is. Meaning the libname
doesn't include the full path, like it's usual with native cmake
detection. The path comes separately and needs to be rolled separately.
For this we add it to targets via `link_directories()`. We also keep tab
of them in `CURL_LIBDIRS` and use that in `libcurl.pc`. Feature checks
also need to receive these paths. CMake doesn't offer
a `CMAKE_REQUIRED_*` variable for this purpose, only
a `CMAKE_REQUIRED_LINK_OPTIONS` accepting raw linker flags. Add a macro
to convert a list of paths to linker options to solve it. wolfSSL
requires this for now.

Closes #15193
vszakats added a commit to curl/curl that referenced this pull request Nov 14, 2024
The idea of linking dependencies found to `libcurl.pc` turns out not
to work in practice in some cases.

Specifically: gss, ldap, mbedtls, libmsh3, rustls

A `.pc` may not work or be missing for a couple of reasons:
- not all build methods generate it: mbedTLS, Rustls
- generated file is broken: msh3
  Ref: nibanks/msh3#225
- installed package flavour isn't shipping with one:
  FreeBSD GSS, OmniOS LDAP, macOS LDAP

The effect of such issues shall be subtle in theory, because
`libcurl.pc` normally lists these dependencies in the `Requires.private`
section meant for static linking. But, e.g. `pkg-config --exists`
requires these to be present, and builds sometimes use this check
regardless of build type. This bug is not present in `pkgconf`; it only
checks for them when `--static` is also passed.

Fix these by adding affected `.pc` references to `libcurl.pc` only when
we detected the dependency via `pkg-config`.

There are a few side-effects of this solution:
- references are never added for dependencies where curl doesn't
  implement `pkg-config` detection. These are:
  - autotools: ldap, mbedtls, msh3
  - cmake: ldap (pending #15273)
- generated `libcurl.pc` depends on the build-time environment.
- generated `libcurl.pc` depends on curl build tool (cmake, autotools).
- generated `libcurl.pc` depends on curl build implementation details.

Make an exception for GNU GSS, where I blindly guess that `gss.pc` is
always available, as no issues were reported.

Other, not mentioned, dependencies continue to be added regardless
of the detection method.

Reported-by: Harmen Stoppels, Thomas, Daniel Engberg, Andy Fiddaman
Fixes #15469
Fixes #15507
Fixes #15535
Fixes #15163 (comment)
Closes #15573
@nibanks nibanks merged commit 8c7e75c into nibanks:main Nov 25, 2024
21 checks passed
@vszakats vszakats deleted the fix-pc-1 branch November 25, 2024 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants